Implement is_articulation to check if vertex is articulation point
#387
Conversation
|
Timing-wise, julia> g = path_graph(5)
julia> articulation(g) # [2, 3, 4]
julia> @btime 2 ∈ articulation($g) # 209.656 ns (8 allocations: 704 bytes)
julia> @btime is_articulation($g, 2) # 122.850 ns (4 allocations: 464 bytes)
julia> g = path_graph(15)
julia> articulation(g) # [2:14...]
julia> @btime 9 ∈ articulation($g) # 453.853 ns (10 allocations: 2.16 KiB)
julia> @btime is_articulation($g, 9) # 245.286 ns (4 allocations: 624 bytes) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #387 +/- ##
==========================================
- Coverage 97.31% 97.30% -0.02%
==========================================
Files 120 120
Lines 6954 6965 +11
==========================================
+ Hits 6767 6777 +10
- Misses 187 188 +1 ☔ View full report in Codecov by Sentry. |
|
Bump. |
|
Bump :) |
| name = "Graphs" | ||
| uuid = "86223c79-3864-5bf0-83f7-82e725a168b6" | ||
| version = "1.11.1" | ||
| version = "1.11.2" |
There was a problem hiding this comment.
merge default branch into this PR, the version has evolved since
| s = Vector{Tuple{T,T,T}}() | ||
| low = zeros(T, nv(g)) | ||
| pre = zeros(T, nv(g)) |
There was a problem hiding this comment.
If the idea is to call is_articulation several times in a row for different vertices, these allocations might be reusable? Maybe we should expose articulation_dfs!?
There was a problem hiding this comment.
Sure, that'd be alright with me also. The _dfs part is an implementation detail though, so maybe this should just be called is_articulation!, taking work-arrays for pre and low then? Let me know what you think and I'll change to that.
There was a problem hiding this comment.
Sounds good, but then we should at the very least explain how to initialize these arguments (and ideally what they mean)
There was a problem hiding this comment.
See also discussion in #407 (comment): maybe we should just be calling these work_buffer1 and work_buffer2 (adding some comments to explain in the code - or reassigning them to more aptly named variables) to avoid exposing unnecessary complications and implementation details to users.
| if !isnothing(is_articulation_pt) | ||
| if pre[u] != 0 | ||
| return is_articulation_pt | ||
| end | ||
| end |
There was a problem hiding this comment.
Can you explain this shortcut in a comment?
Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com>
2 participants
This factors out the DFS step from
articulationinto a separate functionarticulation_dfs!and then uses this new function to implementis_articulation(g, v)which checks whethervis an articulation point ofgwithout necessarily computing all the articulation points.The branching on
isnothing(is_articulation_pts)inarticulation_dfs!is a bit inelegant - as is the fact thatarticulation_dfs!doesn't mutate ifis_articulation_pts === nothing, but it seemed the simplest way to avoid code duplication.Aside from these additions, I revised the doc string of
articulation(g)a bit (e.g., it 1. previously misleadingly stated thatghad to be connected; it does not; and 2. it changed nomenclature from "articulation point" to "cut vertex" mid-sentence).